Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix sourcepos, add missing extension tests and add sourcepos tests #223

Open
wants to merge 27 commits into
base: master
Choose a base branch
from

Conversation

martincizek
Copy link

Fix #222. This PR:

Quality assurance:

  • The tests should be thourough enough.
  • Also introduced new tests for extensions (strikethrough, tables).
  • make test passing
  • make leakcheck is passing.
  • As a significant part of the added code is only run when CMARK_OPT_SOURCEPOS is set, I've tried to set and unset this flag for parser only (not for the renderer), by hardcoding this in cmark_parser_new_with_mem. In both cases, all tests pass except for a few tests failing on sourcepos mismatch (which was expected).
  • We've performed test on ~250k real-world Markdown texts with the utility GFM editor and matched the detected sourcepos with the source using regexps (only for the tags we were interested in).

Performance - I wasn't able to measure any difference:

  • Default setup:
    master branch:        mean = 0.8915, median = 0.8800, stdev = 0.0230
    fix-sourcepos branch: mean = 0.8875, median = 0.8800, stdev = 0.0165
    
  • When CMARK_OPT_SOURCEPOS was hardcoded in cmark_parser_new_with_mem:
    master branch:        mean = 0.8935, median = 0.8900, stdev = 0.0160
    fix-sourcepos branch: mean = 0.8945, median = 0.8900, stdev = 0.0170
    

I'd be glad if this is going to be fixed, as we now have to maintain own fork with fixed cmark-gfm and a our fork of commonmarker.

And any feedback would be welcome!

chriszielinski and others added 27 commits May 4, 2021 23:02
…ejected from header (sourcepos of such paragraph, the table and all nested elements are broken).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sourcepos broken for tables and inlines and confusing CMARK_OPT_SOURCEPOS behavior
2 participants